#!/bin/bash
# Check for any remaining references to bbl_format in the codebase
echo "=== Checking for remaining bbl_format references ==="
rg -n 'bbl_format' --type rust

echo -e "\n=== Checking what the crate publicly exports from lib.rs ==="
cat src/lib.rs

echo -e "\n=== Checking what parser module exports ==="
cat src/parser/mod.rs

echo -e "\n=== Checking main.rs imports from the crate ==="
rg -n '^use bbl_parser::' src/main.rs

echo -e "\n=== Checking if there are any duplicate sign_extend implementations ==="
rg -n 'fn sign_extend' --type rust

echo -e "\n=== Checking if there are duplicate apply_predictor or parse_frame_data implementations ==="
rg -n 'fn (apply_predictor|parse_frame_data)' --type rust

echo -e "\n=== Verify BBLDataStream is only in one place ==="
rg -n 'struct BBLDataStream' --type rust

echo -e "\n=== Check if CLI main.rs has any local parsing logic (should be minimal) ==="
wc -l src/main.rs

#!/bin/bash
# Check what main.rs contains - looking for any parsing logic that should be in the crate
echo "=== Checking main.rs for potential local parsing logic ==="
rg -n 'fn (parse_|read_|decode_|apply_)' src/main.rs | head -20

echo -e "\n=== Check if main.rs has any BBL-specific encoding/decoding constants defined locally ==="
rg -n 'const (ENCODING|PREDICT)_' src/main.rs

echo -e "\n=== Verify the issue #11 objectives ==="
gh issue view 11 --json title,body --jq '.body' | head -80

echo -e "\n=== Check what functions main.rs defines ==="
ast-grep --pattern 'fn $FUNC($_) $$$' src/main.rs | head -30

#!/bin/bash
# Check if CLI parse_i_frame and similar functions use the crate's parse_frame_data
echo "=== Checking if parse_i_frame uses crate's parse_frame_data ==="
sed -n '2299,2329p' src/main.rs

echo -e "\n=== Checking if parse_s_frame uses crate's parse_frame_data ==="
sed -n '2330,2392p' src/main.rs

echo -e "\n=== Check parse_frames function to see overall structure ==="
sed -n '1655,1700p' src/main.rs

echo -e "\n=== Verify no local ENCODING or predictor logic in main.rs ==="
rg -n '(ENCODING_|PREDICT_)' src/main.rs | head -20

echo -e "\n=== Check if there's any local decode/apply logic in main.rs ==="
rg -n 'decode_frame_field|apply_predictor' src/main.rs

#!/bin/bash
# Check if P-frames use the crate's parse_frame_data
echo "=== Looking for P-frame handling in parse_frames ==="
sed -n '1850,1950p' src/main.rs

echo -e "\n=== Check if I/S/H frames in main.rs call parse_frame_data from crate ==="
rg -A 5 "'I' =>" src/main.rs | head -20
rg -A 5 "'S' =>" src/main.rs | head -20

echo -e "\n=== Look for usage of parse_frame_data function in main.rs ==="
rg -n 'parse_frame_data\(' src/main.rs

echo -e "\n=== Check Phase 2 requirement from issue #11 ==="
gh issue view 11 --json body --jq '.body' | grep -A 10 "Phase 2"

#!/bin/bash
# Check if parse_i_frame, parse_s_frame, parse_h_frame in main.rs are actually used
echo "=== Check if parse_i_frame is called anywhere ==="
rg -n 'parse_i_frame\(' src/main.rs

echo -e "\n=== Check if parse_s_frame is called anywhere ==="
rg -n 'parse_s_frame\(' src/main.rs

echo -e "\n=== Check if parse_h_frame is called anywhere ==="
rg -n 'parse_h_frame\(' src/main.rs

echo -e "\n=== Check if parse_e_frame is called anywhere ==="
rg -n 'parse_e_frame\(' src/main.rs

echo -e "\n=== Check the entire frame type match statement to see what's called ==="
sed -n '1710,2050p' src/main.rs | grep -A 30 "match frame_type"

#!/bin/bash
# Final verification - check if parse_i_frame is truly dead code
echo "=== Verify parse_i_frame is never called ==="
rg -n 'parse_i_frame\(' src/main.rs

echo -e "\n=== Check what was in bbl_format.rs according to git history ==="
git log --all --full-history --format="%H" -- src/bbl_format.rs | head -1 | xargs -I {} git show {}:src/bbl_format.rs | head -50

echo -e "\n=== Summary: What are the CLI-specific parse functions still in main.rs? ==="
rg -n '^fn parse_[a-z_]+\(' src/main.rs

echo -e "\n=== Verify all three phases from issue #11 ==="
echo "Phase 1 - Helpers: sign_extend functions"
rg -c 'pub fn sign_extend' src/parser/helpers.rs

echo -e "\nPhase 2 - parse_frame_data with debug parameter"
rg -n 'pub fn parse_frame_data' src/parser/frame.rs | head -3

echo -e "\nPhase 3 - bbl_format.rs deleted?"
ls src/bbl_format.rs 2>&1 || echo "✅ bbl_format.rs successfully deleted"

echo -e "\n=== Check test results ==="
cargo test --all 2>&1 | tail -20

#!/bin/bash
# Check if S, H, E frame parsing should also use crate functions
echo "=== Check if crate has parse_s_frame, parse_h_frame, parse_e_frame ==="
rg -n 'pub fn parse_(s|h|e)_frame' src/parser/

echo -e "\n=== Check what frame types use crate parse_frame_data vs local functions ==="
echo "I-frames (should use crate):"
sed -n '1740,1820p' src/main.rs | grep -A 15 "'I' =>"

echo -e "\nP-frames (should use crate):"
sed -n '1820,1900p' src/main.rs | grep -A 15 "'P' =>"

echo -e "\nS-frames (currently use local parse_s_frame):"
sed -n '1940,1970p' src/main.rs

echo -e "\nH-frames (currently use local parse_h_frame):"
sed -n '1975,2005p' src/main.rs

echo -e "\n=== Check if S and H frames could use parse_frame_data ==="
echo "Looking at S-frame structure:"
sed -n '2330,2400p' src/main.rs

#!/bin/bash
# Compare CLI parse_h_frame with crate parse_h_frame
echo "=== CLI parse_h_frame implementation ==="
sed -n '2393,2435p' src/main.rs

echo -e "\n=== Crate parse_h_frame implementation ==="
sed -n '1,50p' src/parser/gps.rs

echo -e "\n=== CLI parse_e_frame implementation ==="
sed -n '2436,2500p' src/main.rs

echo -e "\n=== Crate parse_e_frame implementation ==="
sed -n '38,80p' src/parser/event.rs

echo -e "\n=== Final check: Is there a parse_s_frame in the crate? ==="
rg -n 'fn parse_s_frame' src/parser/

echo -e "\n=== Check if parse_i_frame is truly unused ==="
ast-grep --pattern 'parse_i_frame($$$)' src/main.rs

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: LICENSE_COMMERCIAL:1-4
Timestamp: 2025-08-29T19:52:05.099Z
Learning: nerdCopter prefers to avoid publishing personal information in license files for privacy and security reasons, as they are an individual maintainer rather than a company.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: README.md:520-521
Timestamp: 2025-08-29T19:53:41.354Z
Learning: nerdCopter uses AGPL-3.0-or-later licensing for the bbl_parser project with a dual-licensing approach that includes a separate commercial license option.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: CONTRIBUTING.md:9-14
Timestamp: 2025-08-21T20:25:45.741Z
Learning: nerdCopter prefers to keep CLA language general using "project maintainer" rather than specifying a legal entity name, as they are an individual maintainer without an associated company.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: LICENSE_COMMERCIAL:0-0
Timestamp: 2025-08-29T20:15:04.624Z
Learning: nerdCopter prefers clear positive indicators (✅) when describing fixes rather than using ❌ symbols which can be confusing when describing what was corrected in a positive context.

#!/bin/bash
# Gather detailed information for the follow-up issue

echo "=== 1. Analyze parse_i_frame (dead code) ==="
echo "Location and usage check:"
rg -n 'fn parse_i_frame' src/main.rs
rg -n 'parse_i_frame\(' src/main.rs
echo -e "\nFunction implementation:"
sed -n '2299,2329p' src/main.rs

echo -e "\n=== 2. Compare parse_s_frame implementations ==="
echo "CLI version (src/main.rs):"
sed -n '2330,2392p' src/main.rs
echo -e "\nCrate version (src/parser/frame.rs):"
sed -n '600,700p' src/parser/frame.rs
echo -e "\nCLI call site:"
sed -n '1940,1955p' src/main.rs

echo -e "\n=== 3. Compare parse_h_frame implementations ==="
echo "CLI version (src/main.rs):"
sed -n '2393,2435p' src/main.rs
echo -e "\nCrate version (src/parser/gps.rs):"
cat src/parser/gps.rs
echo -e "\nCLI call site:"
sed -n '1975,1990p' src/main.rs

echo -e "\n=== 4. Compare parse_e_frame implementations ==="
echo "CLI version (src/main.rs):"
sed -n '2436,2500p' src/main.rs
echo -e "\nCrate version (src/parser/event.rs):"
cat src/parser/event.rs
echo -e "\nCLI call site:"
sed -n '2120,2135p' src/main.rs

echo -e "\n=== 5. Count total lines of duplication ==="
echo "parse_i_frame lines:"
sed -n '2299,2329p' src/main.rs | wc -l
echo "parse_s_frame lines:"
sed -n '2330,2392p' src/main.rs | wc -l
echo "parse_h_frame lines:"
sed -n '2393,2435p' src/main.rs | wc -l
echo "parse_e_frame lines:"
sed -n '2436,2500p' src/main.rs | wc -l

#!/bin/bash
# Create the GitHub issue with detailed analysis

# First, let's verify read_neg_14bit implementation
echo "=== Verify read_neg_14bit equivalence ==="
rg -n 'fn read_neg_14bit' src/parser/stream.rs -A 5

echo -e "\n=== Check if CLI parse_e_frame is complete ==="
sed -n '2436,2600p' src/main.rs | tail -40

echo -e "\n=== Count exact lines to be removed ==="
echo "Total lines in functions to be analyzed/removed:"
sed -n '2299,2500p' src/main.rs | wc -l

#!/bin/bash
# Create the follow-up GitHub issue with detailed analysis

gh issue create \
  --title "Complete CLI-to-Crate Unification: Phase 4 (S/H/E Frame Parsers)" \
  --assignee nerdCopter \
  --body "## Context

Follow-up to PR #12 which successfully completed issue #11 (Phases 1-3). This issue addresses **remaining code duplication** discovered during PR review.

**Reference**: [PR #12 comment thread](https://github.com/nerdCopter/bbl_parser/pull/12#discussion_r1887384567)

---

## Summary

PR #12 eliminated **471 net lines** of duplication by unifying helpers, \`parse_frame_data\`, and \`BBLDataStream\`. However, **~202 additional lines** of frame parser duplication remain in \`src/main.rs\`:

1. **Dead code**: \`parse_i_frame\` (31 lines) — never called
2. **Duplicated**: \`parse_s_frame\` (63 lines) — **needs verification** (has TAG2_3S32 handling)
3. **Duplicated**: \`parse_h_frame\` (43 lines) — nearly identical to crate version
4. **Duplicated**: \`parse_e_frame\` (65 lines) — crate version is MORE comprehensive

---

## Detailed Analysis

### 1. Dead Code: \`parse_i_frame\` (Lines 2299-2329)

**Status**: 🔴 Dead code — never called

**Evidence**:
\`\`\`bash
# Function defined but never invoked
\$ rg 'parse_i_frame\\(' src/main.rs
2299:fn parse_i_frame(  # definition only, no call sites
\`\`\`

I-frames use the crate's \`parse_frame_data\` directly (line 1750):
\`\`\`rust
'I' => {
    // ... existing setup ...
    parse_frame_data(
        &mut stream,
        &header.i_frame_def,
        &mut i_frame_values,
        // ...
    )?;
}
\`\`\`

**Action**: Delete lines 2299-2329.

---

### 2. Duplication: \`parse_s_frame\` (Lines 2330-2392)

**Status**: ⚠️ **Verification required** — implementations differ

**Key Difference**:
- **CLI version**: Handles \`ENCODING_TAG2_3S32\` (reads 3 fields at once)
- **Crate version** (\`src/parser/frame.rs:600\`): Does NOT handle TAG2_3S32

**CLI implementation snippet**:
\`\`\`rust
ENCODING_TAG2_3S32 => {
    let mut values = [0i32; 8];
    stream.read_tag2_3s32(&mut values)?;
    for j in 0..3 {
        if field_index + j < frame_def.fields.len() {
            let current_field = &frame_def.fields[field_index + j];
            data.insert(current_field.name.clone(), values[j]);
        }
    }
    field_index += 3;
}
\`\`\`

**Crate implementation** (src/parser/frame.rs:600):
\`\`\`rust
fn parse_s_frame(...) -> Result<HashMap<String, i32>> {
    let mut data = HashMap::new();
    for field in &frame_def.fields {
        let value = match field.encoding {
            ENCODING_SIGNED_VB => stream.read_signed_vb()?,
            ENCODING_UNSIGNED_VB => stream.read_unsigned_vb()? as i32,
            ENCODING_NEG_14BIT => stream.read_neg_14bit()?,
            ENCODING_NULL => 0,
            _ => { /* fallback */ }
        };
        data.insert(field.name.clone(), value);
    }
    Ok(data)
}
\`\`\`

**Migration Strategy**:

**Option A** (Safer): Enhance crate's \`parse_s_frame\` first
1. Add TAG2_3S32 handling to \`src/parser/frame.rs:parse_s_frame\`
2. Match CLI's field iteration logic (\`field_index\` increment by group size)
3. Add test to verify S-frame parsing with TAG2_3S32 encoding
4. Replace CLI call site (line 1946) with crate import
5. Delete CLI implementation (lines 2330-2392)

**Option B** (Riskier): Verify TAG2_3S32 is never used in S-frames
1. Analyze test logs to confirm S-frames never use TAG2_3S32
2. If confirmed unused, directly replace with crate version
3. Add regression test to catch TAG2_3S32 in S-frames if it appears

**Recommended**: **Option A** — preserving existing behavior is safer.

---

### 3. Duplication: \`parse_h_frame\` (Lines 2393-2435)

**Status**: ✅ Safe to replace — implementations are equivalent

**Comparison**:
| Aspect | CLI (main.rs:2393) | Crate (gps.rs:22) |
|--------|-------------------|-------------------|
| Structure | Identical | Identical |
| ENCODING_NEG_14BIT | \`-(sign_extend_14bit(...))\` | \`read_neg_14bit()\` (same result) |
| Error handling | \`unwrap_or(0)\` fallback | \`unwrap_or(0)\` fallback |
| Debug output | Same format | Same format |

**Migration**:
1. Update \`src/main.rs\` line 1979 call site:
   \`\`\`rust
   // Before
   if let Ok(data) = parse_h_frame(&mut stream, &header.h_frame_def, debug) {
   
   // After
   use bbl_parser::parser::gps::parse_h_frame;
   if let Ok(data) = parse_h_frame(&mut stream, &header.h_frame_def, debug) {
   \`\`\`
2. Delete CLI implementation (lines 2393-2435)

---

### 4. Duplication: \`parse_e_frame\` (Lines 2436-2500)

**Status**: ✅ Safe to replace — **crate version is superior**

**Why Crate Version is Better**:
- Handles additional event types: 10, 11, 12, 13, 14
- Includes \`parse_inflight_adjustment\` helper (DRY principle)
- More comprehensive event type coverage
- Better documented with module-level comments

**Event Types Comparison**:
| Event Type | CLI Support | Crate Support | Notes |
|------------|-------------|---------------|-------|
| 0-6 | ✅ | ✅ | Identical |
| 10-12 | ❌ | ✅ | UNUSED autotune variants |
| 13 | ❌ | ✅ | Inflight adjustment (alternate numbering) |
| 14 | ❌ | ✅ | Logging resume (newer numbering) |
| 30, 255 | ✅ | ✅ | Identical |

**Migration**:
1. Update \`src/main.rs\` line 2123 call site:
   \`\`\`rust
   // Before
   if let Ok(mut event_frame) = parse_e_frame(&mut stream, debug) {
   
   // After
   use bbl_parser::parser::event::parse_e_frame;
   if let Ok(mut event_frame) = parse_e_frame(&mut stream, debug) {
   \`\`\`
2. Delete CLI implementation (lines 2436-2500)

---

## Step-by-Step Migration Plan

### Phase 4a: Remove Dead Code (Low Risk)
**Goal**: Delete unused \`parse_i_frame\`

\`\`\`bash
# 1. Verify function is never called
rg 'parse_i_frame\\(' src/main.rs  # Should only show definition

# 2. Delete lines 2299-2329
sed -i '2299,2329d' src/main.rs

# 3. Validate
cargo build
cargo test
\`\`\`

**Success Criteria**: Build passes, all 50 tests pass

---

### Phase 4b: Replace H-frame Parser (Low Risk)
**Goal**: Use crate's \`parse_h_frame\`

\`\`\`bash
# 1. Add import to main.rs (near top with other bbl_parser imports)
# Add: use bbl_parser::parser::gps::parse_h_frame;

# 2. Verify call site uses imported version (line ~1979)
rg -A 3 'parse_h_frame' src/main.rs

# 3. Delete CLI implementation (lines 2393-2435)
# Note: Line numbers shift after previous deletion, recalculate

# 4. Validate
cargo build
cargo test --all

# 5. A/B test: Compare CSV output
cargo run -- --input tests/sample.bbl --output /tmp/before.csv
# (after changes)
cargo run -- --input tests/sample.bbl --output /tmp/after.csv
diff /tmp/before.csv /tmp/after.csv  # Should be empty
\`\`\`

**Success Criteria**: 
- Build passes
- All tests pass
- CSV outputs identical

---

### Phase 4c: Replace E-frame Parser (Low Risk)
**Goal**: Use crate's superior \`parse_e_frame\`

\`\`\`bash
# 1. Add import to main.rs
# Add: use bbl_parser::parser::event::parse_e_frame;

# 2. Verify call site (line ~2123)
rg -A 3 'parse_e_frame' src/main.rs

# 3. Delete CLI implementation (lines 2436-2500, adjusted for previous deletions)

# 4. Validate
cargo build
cargo test --all

# 5. Verify event parsing works (especially event types 10-14)
cargo run -- --input tests/sample_with_events.bbl --output /tmp/events.csv
# Check for any new event types captured
\`\`\`

**Success Criteria**:
- Build passes
- All tests pass
- Event frames parsed correctly (check stats output)

---

### Phase 4d: Replace S-frame Parser (⚠️ Higher Risk)
**Goal**: Unify \`parse_s_frame\` after verifying TAG2_3S32 handling

**Step 1: Investigate TAG2_3S32 Usage**
\`\`\`bash
# Check if any test logs use TAG2_3S32 in S-frames
for f in tests/*.bbl; do
  echo \"Checking \$f\"
  cargo run -- --input \"\$f\" --debug 2>&1 | grep -i 'S-frame.*TAG2_3S32'
done

# Check frame definitions
rg 'TAG2_3S32' src/
\`\`\`

**Step 2a: If TAG2_3S32 is used** (Safer Path)
\`\`\`bash
# 1. Enhance crate's parse_s_frame in src/parser/frame.rs
#    Add TAG2_3S32 handling matching CLI implementation:
\`\`\`
\`\`\`rust
// In src/parser/frame.rs:parse_s_frame, change to:
fn parse_s_frame(...) -> Result<HashMap<String, i32>> {
    let mut data = HashMap::new();
    let mut field_index = 0;
    
    while field_index < frame_def.fields.len() {
        let field = &frame_def.fields[field_index];
        
        match field.encoding {
            ENCODING_SIGNED_VB => { /* existing */ }
            ENCODING_UNSIGNED_VB => { /* existing */ }
            ENCODING_NEG_14BIT => { /* existing */ }
            ENCODING_TAG2_3S32 => {
                // NEW: Handle 3 fields at once
                let mut values = [0i32; 8];
                stream.read_tag2_3s32(&mut values)?;
                for j in 0..3 {
                    if field_index + j < frame_def.fields.len() {
                        let current_field = &frame_def.fields[field_index + j];
                        data.insert(current_field.name.clone(), values[j]);
                    }
                }
                field_index += 3;
                continue;
            }
            ENCODING_NULL => { /* existing */ }
            _ => { /* existing fallback */ }
        };
        
        data.insert(field.name.clone(), value);
        field_index += 1;
    }
    
    Ok(data)
}
\`\`\`
\`\`\`bash
# 2. Test enhanced crate function
cargo test

# 3. Replace CLI call site (line ~1946)
# Add import: use bbl_parser::parser::frame::parse_s_frame;

# 4. Delete CLI implementation

# 5. Validate with all test logs
for f in tests/*.bbl; do
  cargo run -- --input \"\$f\" --output /tmp/test_output.csv
  # Compare with known-good output if available
done
\`\`\`

**Step 2b: If TAG2_3S32 is NOT used** (Simpler Path)
\`\`\`bash
# 1. Add import to main.rs
# Add: use bbl_parser::parser::frame::parse_s_frame;

# 2. Replace call site (line ~1946)

# 3. Delete CLI implementation

# 4. Add regression test to catch TAG2_3S32 if it appears:
#    Add test that fails if TAG2_3S32 encoding is found in S-frames
\`\`\`

**Success Criteria**:
- Build passes
- All 50 tests pass
- CSV outputs identical across all test logs
- No S-frame parsing errors in debug output

---

## Validation Checklist

After completing all phases:

- [ ] \`cargo build\` — no errors
- [ ] \`cargo test --all\` — all 50 tests pass
- [ ] \`cargo clippy\` — zero warnings
- [ ] \`rg 'fn parse_[ishe]_frame' src/main.rs\` — returns nothing
- [ ] \`tokei src/main.rs\` — verify ~200 lines removed
- [ ] A/B test all test logs — CSV outputs identical
- [ ] Visual inspection: \`rg 'parse_[ISPHE]' src/main.rs\` — only crate imports remain

---

## Rollback Plan

If any phase fails validation:

\`\`\`bash
# Discard changes and return to current state
git checkout src/main.rs
git checkout src/parser/frame.rs  # if modified
cargo test --all  # Verify stable state
\`\`\`

---

## Expected Benefits

- **Code reduction**: ~202 lines removed from CLI
- **Maintenance**: Single source of truth for frame parsing
- **Functionality**: Gain event types 10-14 in E-frame parsing
- **Consistency**: All frame parsing uses crate implementations

---

## Dependencies

- Requires PR #12 to be merged first (provides unified parse_frame_data and BBLDataStream)

---

## Notes

- Phases 4a-c are **low risk** (straightforward replacements)
- Phase 4d (S-frames) requires **verification** due to TAG2_3S32 handling difference
- Recommend committing each phase separately for easy rollback
- All changes preserve behavior (no algorithm modifications)

---

## References

- Original unification issue: #11
- Completed phases (1-3): PR #12
- Analysis comment: https://github.com/nerdCopter/bbl_parser/pull/12#discussion_r1887384567"

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: LICENSE_COMMERCIAL:1-4
Timestamp: 2025-08-29T19:52:05.099Z
Learning: nerdCopter prefers to avoid publishing personal information in license files for privacy and security reasons, as they are an individual maintainer rather than a company.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: README.md:520-521
Timestamp: 2025-08-29T19:53:41.354Z
Learning: nerdCopter uses AGPL-3.0-or-later licensing for the bbl_parser project with a dual-licensing approach that includes a separate commercial license option.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: CONTRIBUTING.md:9-14
Timestamp: 2025-08-21T20:25:45.741Z
Learning: nerdCopter prefers to keep CLA language general using "project maintainer" rather than specifying a legal entity name, as they are an individual maintainer without an associated company.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: LICENSE_COMMERCIAL:0-0
Timestamp: 2025-08-29T20:15:04.624Z
Learning: nerdCopter prefers clear positive indicators (✅) when describing fixes rather than using ❌ symbols which can be confusing when describing what was corrected in a positive context.